Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api: add eds to config dump #11425

Merged
merged 2 commits into from
Jun 4, 2020
Merged

api: add eds to config dump #11425

merged 2 commits into from
Jun 4, 2020

Conversation

Rainton
Copy link
Contributor

@Rainton Rainton commented Jun 3, 2020

Commit Message:
add EndpointsConfigDump message to support EDS in config_dump.proto
(not implemented in Envoy)

Additional Description:
Risk Level: Low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A

This is the first step to solve #3362

Signed-off-by: Yutong Li [email protected]
/cc @fuqianggao @alexburnos

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/.

🐱

Caused by: #11425 was opened by Rainton.

see: more, trace.

@Rainton
Copy link
Contributor Author

Rainton commented Jun 3, 2020

I will implement this in Envoy later.

@Rainton Rainton changed the title api: add eds to config dump #3362 api: add eds to config dump Jun 3, 2020

// [#not-implemented-hide:]
// Envoy's EDS implementation *will* fill this message with all currently known endpoints. Endpoint
// configuration information can be used to recreate an Envoy configuration by populating all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought, since number of endpoints could potentially be very large, do we want to dump out all of them or should we have some upper limit.

Would like to see what the API reviewer thinks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the admin handler, this could be just a query param. Could be worth adding as needed.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@repokitteh-read-only repokitteh-read-only bot removed the api label Jun 4, 2020
@htuch htuch merged commit 192cd13 into envoyproxy:master Jun 4, 2020
htuch pushed a commit that referenced this pull request Jun 22, 2020
EDS config has been added to config_dump in #11425 and implemented in #11577 .

Risk Level: Low
Testing: N/A

Signed-off-by: Yutong Li <[email protected]>
songhu pushed a commit to songhu/envoy that referenced this pull request Jun 25, 2020
EDS config has been added to config_dump in envoyproxy#11425 and implemented in envoyproxy#11577 .

Risk Level: Low
Testing: N/A

Signed-off-by: Yutong Li <[email protected]>
yashwant121 pushed a commit to yashwant121/envoy that referenced this pull request Jul 24, 2020
Add EndpointsConfigDump message to support EDS in config_dump.proto
(not implemented in Envoy)

Additional Description:
Risk Level: Low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A

This is the first step to solve envoyproxy#3362

Signed-off-by: Yutong Li <[email protected]>
Signed-off-by: yashwant121 <[email protected]>
yashwant121 pushed a commit to yashwant121/envoy that referenced this pull request Jul 24, 2020
EDS config has been added to config_dump in envoyproxy#11425 and implemented in envoyproxy#11577 .

Risk Level: Low
Testing: N/A

Signed-off-by: Yutong Li <[email protected]>
Signed-off-by: yashwant121 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants